-
Notifications
You must be signed in to change notification settings - Fork 72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
booster-http refactor - trustless only, improved compression, logging & e2e testing w/ trusted #1676
Conversation
Thanks for putting this together @rvagg We actually have a PR that simplifies the parameters to booster-http, and takes advantage of the boxo gateway's trustless gateway: feat: add support for deserialized response How would pulling in this frisbii implementation differ from using the boxo gateway? |
This direction was well-received at the fil dev summit last week.
@rvagg - what's needed to get this to a point where it could be ready for review? |
7a42013
to
4ef62b7
Compare
Changed so this removes The whole frisbii directory will go when the ipnisync branch gets merged and we get all the updated dependencies. For now it's an up to date fork of the frisbii components being used. |
removed the frisbii fork now that the libipni branch is merged and dependencies updated so we have no conflicts! |
Does this require docs changes? If yes, then can we add some rough documentation to this PR and I can update boost docs. |
yes, working on it, first I'll do so by getting an integration test in here that uses bifrost-gateway to demonstrate that it can be used on top of this, then we can add that to the docs as well so all options are covered. |
includes a full e2e test now that involves bifrost-gateway and lassie to perform some trustless and trusted fetching, these are both installed on demand and are not direct dependencies; I have some cleanup to do before this is ready but it's mostly there now |
4e4b295
to
3b07968
Compare
Rebased and merged in #1736 and #1737 to this and simplified the commits significantly. There's 4 commits now - the main switch to frisbii, the fixes for compression and addition of The bulk of the changes here are the addition of extra tests, including an e2e one involving bifrost-gateway to exercise trusted gateway support, and the refactoring of the piece handler does create some annoying diff noise, but it's last in the commit list so you could review that separately to avoid the noise—there are a bunch of changes to the piece handler to support logging and compression that are masked by the refactor. Further comments about compression are in #1736 and logging, including example output, is in #1737. |
7a9f439
to
95a411c
Compare
Updated to frisbii@v0.4.1 for a minor byte-range fix but this also brings in some other deps - notably graphsync v0.15, which thankfully doesn't cause problems here now because of the libp2p update (which was the main difference between the head versions on v0.14 and v0.15) and go-graphsync is only used for testing, with boost-graphsync used for builds here. Also brings in various golang.org/x/ upgrades which come along with golang.org/x/net@v0.17.0 for the http/2 security fixes ipld/frisbii#63 |
Pending AI to merge in main:
|
* print info msgs on removed options * add full e2e+bifrost-gateway test
… GzipHandler usage
…d http log output
DEVNET Testing:
|
Tested on filcollins:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All tests are complete. I added some fixes for devnet.
I think we are good to merge.
This is a bit of a PoC at the moment; enables a mini Trustless Gateway implementation when you only want to serve CARs through
/ipfs/
. Uses Frisbii but I've copied in the two main modules that are used into this PR because of the libp2p dependency pain that gets stuck at ipni dependencies in Frisbii which are not required here. So cmd/booster-http/frisbii/* are (mostly) unmodified copies from github.com/ipld/frisbii - we only need the handler and the CAR streamer for this.The transparent switch in implementations by subtle CLI argument changes is probably not optimal for users, but I've got a big integration test in here that pulls in the big trustless pathing test fixture that exercises a whole lot of trustless path retrievals for a fixed 20Mb CAR (which we import across multiple deals here) and it gets run across both implementations in here with the same boost backend, just two different booster-http frontends.
The reason this might be useful are that (1) it's super minimal and very lightweight; there's no intermediate caching of blocks while it builds up state, it just does a traversal straight from the backend blockstore and spits it out as a CAR, (2) it gives us a bit more freedom to experiment with alternative outputs, like what SPARK is after, and (3) gives us useful error output if there's a problem loading the root block, instead of the current empty CAR that we get.